Skip to content

Closes #30. Add the ability to specify custom NameNode name and edits dirs.#40

Merged
xkrogen merged 1 commit into
linkedin:masterfrom
xkrogen:ekrogen-name-edits-dir
May 7, 2018
Merged

Closes #30. Add the ability to specify custom NameNode name and edits dirs.#40
xkrogen merged 1 commit into
linkedin:masterfrom
xkrogen:ekrogen-name-edits-dir

Conversation

@xkrogen

@xkrogen xkrogen commented May 4, 2018

Copy link
Copy Markdown
Collaborator

No description provided.

String datanodeNodeLabelExpression, int datanodesPerCluster, String datanodeLaunchDelay, int namenodeMemoryMB,
int namenodeVirtualCores, String namenodeArgs, String namenodeNodeLabelExpression, int namenodeMetricsPeriod,
Map<String, String> shellEnv) {
String namenodeNameDir, String namenodeEditsDir, Map<String, String> shellEnv) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this constructor is growing huge and a lot have default values, shall we construct this through a builder?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought was to agree with you, but actually, I think it makes sense for it to stay as-is. Since this is just an internal class for encapsulating the result of a number of command-line options, you wouldn't normally call this constructor directly, and in fact the only current usage is within this class (initFromParser()). It does not seem worth creating a builder given this setup.

@xkrogen xkrogen merged commit bfac897 into linkedin:master May 7, 2018
@xkrogen xkrogen deleted the ekrogen-name-edits-dir branch May 7, 2018 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants